Skip to content

Comments

fix: escape html in external oauth error message#28091

Merged
whoiskatrin merged 4 commits intoproductionfrom
sync-docs-pr-841
Feb 11, 2026
Merged

fix: escape html in external oauth error message#28091
whoiskatrin merged 4 commits intoproductionfrom
sync-docs-pr-841

Conversation

@agents-git-bot
Copy link
Contributor

@agents-git-bot agents-git-bot bot commented Feb 4, 2026

Summary

This PR syncs documentation changes from cloudflare/agents#841 which fixed an XSS vulnerability in OAuth error handling for MCP clients.

Changes

  • OAuth callback handler simplified: The customHandler no longer receives MCPClientOAuthResult with error information. It now simply closes the popup window, and errors are handled automatically by the SDK.
  • New error field: Added documentation for the error field in MCPServer type, which stores connection errors for UI display.
  • Security improvement: Error messages from external OAuth providers are automatically escaped to prevent XSS attacks, making them safe to display directly in UIs.
  • Updated examples: All OAuth integration examples now demonstrate the correct pattern of displaying errors from connection state instead of handling them in script alerts.

Files Changed

  • /agents/guides/oauth-mcp-client/ - Updated OAuth integration guide with new error handling patterns
  • /agents/model-context-protocol/mcp-client-api/ - Added error field to MCPServer type documentation

Related

@github-actions github-actions bot added product:agents Build and deploy AI-powered Agents on Cloudflare that can act autonomously. size/s labels Feb 4, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2026

This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:

Pattern Owners
/src/content/docs/agents/ @irvinebroque, @rita3ko, @elithrar, @thomasgauvin, @threepointone, @whoiskatrin, @cloudflare/pcx-technical-writing, @cloudflare/ai-agents, @cloudflare/dev-plat-leads

Updates documentation to reflect security improvements in OAuth error handling:

- Remove MCPClientOAuthResult from customHandler signature (no longer receives error info)
- Document new `error` field in MCPServer type that stores connection errors
- Update examples to display errors from connection state instead of script alerts
- Add note that error messages are automatically escaped to prevent XSS attacks
- Simplify customHandler examples to only close popup (errors handled separately)

Related to cloudflare/agents#841

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@agents-git-bot agents-git-bot bot changed the title docs: Fix XSS vulnerability in MCP OAuth error handling fix: escape html in external oauth error message Feb 4, 2026
@mattzcarey
Copy link
Contributor

looks good to me. @threepointone to go with the PR

@elithrar
Copy link
Collaborator

elithrar commented Feb 6, 2026

/bonk review

@ask-bonk
Copy link
Contributor

ask-bonk bot commented Feb 6, 2026

Here's my review:

Review Summary

This PR correctly documents the security fix from cloudflare/agents#841, which eliminates an XSS vulnerability where error messages from external OAuth providers were interpolated directly into <script> tags via template literals.

Changes look good

  1. oauth-mcp-client.mdx — The customHandler callback is simplified to no longer receive MCPClientOAuthResult. The old pattern of alert('Authorization failed: ${result.authError}') was the XSS vector. Now the handler just closes the popup unconditionally, and errors are surfaced through server.error in the connection state. The React examples are updated accordingly to display server.error when state === "failed".

  2. mcp-client-api.mdx — The error field is added to the MCPServer type definition, and the failed state description now references it. The XSS safety note is clear and accurate.

Issue: Inconsistent customHandler example in mcp-client-api.mdx

The "Custom OAuth callback handling" example at src/content/docs/agents/model-context-protocol/mcp-client-api.mdx:210-218 still uses the old API pattern:

customHandler: (result) => {
    if (result.authSuccess) {
        return new Response("<script>window.close();</script>", {
            headers: { "content-type": "text/html" },
        });
    }
    return new Response("Auth failed", { status: 400 });
},

This is inconsistent with the updated examples in oauth-mcp-client.mdx, where customHandler no longer receives a result parameter. If the SDK change removed MCPClientOAuthResult from the callback signature, this example is now incorrect and should be updated to match the new pattern:

customHandler: () => {
    return new Response("<script>window.close();</script>", {
        headers: { "content-type": "text/html" },
    });
},

This should be fixed before merging to avoid documenting a broken API pattern in the reference page.

Minor note

The import type { MCPClientOAuthResult } from "agents/mcp" removal in the guide is correct — it's no longer used. But the MCP client API page never imported it explicitly in its examples, so no change needed there beyond fixing the customHandler signature.

github run

@elithrar
Copy link
Collaborator

elithrar commented Feb 8, 2026

/bonk resolve merge conflicts and review

- Remove result parameter from customHandler in mcp-client-api.mdx to
  match new SDK API (no longer receives MCPClientOAuthResult)
- Fix server.connectionState -> server.state in client-side example
- Add missing useState import in React examples in oauth-mcp-client.mdx
@ask-bonk
Copy link
Contributor

ask-bonk bot commented Feb 8, 2026

Summary

Merge Conflicts

No real merge conflicts on the two files changed by this PR. The branch has an "unrelated histories" issue with production (likely from how the sync bot created the branch), but the two commits since the last merge (ab4254b, a64f431) don't touch the same files as this PR.

Fixes Applied

Three issues were found and fixed in a single commit:

  1. mcp-client-api.mdx:210-218customHandler example used the old API pattern (result) => { if (result.authSuccess) ... }. Updated to the new no-arg () => { ... } signature, matching the pattern in oauth-mcp-client.mdx. This was the issue flagged in the previous /bonk review.

  2. mcp-client-api.mdx:373 — Client-side React example used server.connectionState which doesn't exist on the MCPServersState type. Fixed to server.state.

  3. oauth-mcp-client.mdx:132,224 — Two React examples used useState without importing it. Added import { useState } from "react" to both code blocks.

Review Notes

  • The core documentation changes (simplified customHandler, new error field, XSS safety notes) are correct and consistent
  • The error: string | null field is properly documented in the MCPServersState type at mcp-client-api.mdx:480
  • The XSS safety note at mcp-client-api.mdx:499 is accurate
  • Minor nit (not fixed): useState<MCPServersState> initial values are missing resourceTemplates — this is a type completeness issue but wouldn't cause runtime errors and is common in doc examples

github run

@cloudflare cloudflare deleted a comment from ask-bonk bot Feb 11, 2026
Merge production into sync-docs-pr-841, resolving conflicts in:
- oauth-mcp-client.mdx: keep simplified customHandler (no MCPClientOAuthResult param),
  error handling via server.error field, XSS escaping notes
- mcp-client-api.mdx: keep error: string | null type, update configureOAuthCallback
  API docs to match new simplified handler, retain callbackPath section from production
@whoiskatrin whoiskatrin merged commit acd6799 into production Feb 11, 2026
9 checks passed
@whoiskatrin whoiskatrin deleted the sync-docs-pr-841 branch February 11, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

product:agents Build and deploy AI-powered Agents on Cloudflare that can act autonomously. size/s

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants